refactor validators to remove duplicate files #5214
Conversation
| File | Before (bytes) | After (bytes) | | --------- | -------------: | ------------: | | `.whl` | 16265568 | 9447645 | | `.tar.gz` | 7666222 | 6615305 |
gvwilson
force-pushed
the
refactor-validators
branch
from
f0fddc2 to
0184bd4
Compare
|
@alexcjohnson if you have time to look this over we'd be grateful for your feedback |
| get_data_validator_instance, | ||
| ) | ||
|
|
||
| # Target Python version for code formatting with Black. | ||
| # Must be one of the values listed in pyproject.toml. | ||
| BLACK_TARGET_VERSIONS = "py38 py39 py310 py311 py312" | ||
| BLACK_TARGET_VERSIONS = "py39 py310 py311 py312" |
There was a problem hiding this comment.
revert this line -- needs to remain the same as what's in pyproject.toml
| BLACK_TARGET_VERSIONS = "py39 py310 py311 py312" | |
| BLACK_TARGET_VERSIONS = "py38 py39 py310 py311 py312" |
| filepath = opath.join(outdir, "validators", "_validators.json") | ||
| with open(filepath, "w") as f: | ||
| f.write(json.dumps(params, indent=4)) | ||
| # f.write(str(params)) |
There was a problem hiding this comment.
Does the knife symbol mean you want to remove this code?
There was a problem hiding this comment.
Ha, yes, sorry @bmaranville -- by 🔪 I mean delete this line 😄 (the commented-out one)
| DataValidator = ValidatorCache.get_validator("", "data") | ||
| FramesValidator = ValidatorCache.get_validator("", "frames") | ||
| LayoutValidator = ValidatorCache.get_validator("", "layout") | ||
| # from .validators import DataValidator, LayoutValidator, FramesValidator |
|
@bmaranville The file Looks like the correct way is to add |
|
Thanks for the PR @bmaranville ! Other than my comments above, looks good to me. The tests are passing and validation seems to be working as usual in the examples I've tried. (@alexcjohnson I'm curious whether you can see any possible pitfalls or edge cases here -- happy to do more testing of specific cases if you have concerns.) |
|
@bmaranville nice work, this is a clever solution! Two small thoughts about it:
true=True
false=False
null=None
v = to the beginning of the file to convert the JSON to Python, and then @emilykl the only thing I'm not 100% confident of is whether this has any type checking implications. I don't think so, I think the only type checking that matters is on |
This PR replaces #5173 by rebasing @bmaranville's changes on top of
main. Please see #5173 for detailed notes about the motivation and changes. The important changes are in:codegen/init.py
codegen/datatypes.py
codegen/validators.py
doc/python/marker-style.md
plotly/_subplots.py
plotly/basedatatypes.py
plotly/figure_factory/_annotated_heatmap.py
plotly/io/_templates.py
plotly/validator_cache.py
I have read through the contributing notes and understand the structure of the package. In particular, if my PR modifies code of
plotly.graph_objects, my modifications concern thecodegenfiles and not generated files.I have added tests (if submitting a new feature or correcting a bug) or
modified existing tests.
For a new feature, I have added documentation examples in an existing or
new tutorial notebook (please see the doc checklist as well).
I have added a CHANGELOG entry if fixing/changing/adding anything substantial.
For a new feature or a change in behaviour, I have updated the relevant docstrings in the code to describe the feature or behaviour (please see the doc checklist as well).